Skip to content

Quote predicate names as identifiers where fragment quoting renders broken SQL - #244

Open
simon-mundy wants to merge 12 commits into
php-db:0.6.xfrom
simon-mundy:fix/predicate-identifier
Open

simon-mundy wants to merge 12 commits into
php-db:0.6.xfrom
simon-mundy:fix/predicate-identifier

Conversation

@simon-mundy

@simon-mundy simon-mundy commented Oct 13, 2026 •

Copy link
Copy Markdown
Member

Merge order: builds on #241's shared IdentifierQuoter, so merge #241 first. Until then this diff also shows #241's changes.

Problem

A predicate's left-hand name is rendered through AbstractPlatform::quoteIdentifierInFragment(). It splits the name on every character outside the platform's word class and quotes each piece, except spaces, dots, * and AS. A name that holds any other delimiter therefore renders as a different, invalid identifier:

Call Today (SQL-92) After
where->equalTo('x-y', 1) "x""-""y" = '1' "x-y" = '1'
where(['t-1.x-y' => 1]) "t""-""1"."x""-""y" = '1' "t-1"."x-y" = '1'
where->like('a"b', …) "a""'""b" LIKE … "a'b" LIKE …
where->isNull('nâme') "n""\xC3""\xA2""me" IS NULL (split bytes) "nâme" IS NULL
having->greaterThan('my col', 1) "my" "col" > '1' "my col" > '1'

This affects these predicates in WHERE and HAVING on Select, and in WHERE on Update and Delete:

  • equalTo, notEqualTo
  • lessThan, lessThanOrEqualTo, greaterThan, greaterThanOrEqualTo
  • like, notLike
  • in, notIn
  • between, notBetween
  • isNull, isNotNull
  • where-array keys

Cause

AbstractSql::processExpression() renders every Argument\Identifier through quoteIdentifierInFragment(), and so does processIdentifiersArgument() for Argument\Identifiers. Fragment quoting is meant for loose SQL, not for a single name. Select columns (#241), ORDER BY names (#243) and Update SET columns (#238) have the same cause.

Fix

Both now quote with IdentifierQuoter::quote(), the shared rule added in #241:

  • Plain and dotted names take a fast path and render exactly as before.
  • Names that fragment quoting renders as written, such as price*qty, a * b and x AS y, keep fragment quoting.
  • A name that fragment quoting would split is quoted whole per qualifier part.
  • Expression syntax that no identifier quoting renders as written throws InvalidArgumentException. Examples are a - b, COUNT(id), and a+b on a platform where - is a word character. An empty name or qualifier part throws too.

The check asks the platform itself, so phpdb-mysql, whose fragment pattern treats - as a word character, keeps `a-b` * `c` unchanged.

The same rule also applies to the other Identifier arguments rendered through processExpression(): identifier-typed Expression parameters and DDL names. DDL names should always be quoted whole; that is listed as a follow-up in #241.

What stays the same

  • Plain names, dotted names, t.*, price*qty, a * b, t.a * t.b, x as y / x AS y and a,b render exactly as before.
  • Expression, Literal and string predicates are untouched, and so are values and string join ON clauses, which call quoteIdentifierInFragment() directly with extra safe words.
  • No public or protected API changes. The new exceptions are named constructors on Sql\Exception\InvalidArgumentException.

Behaviour change

Output changes only for names that rendered invalid SQL before. Those that now throw (expression syntax and empty names) used to render broken SQL that only failed at the database. That includes the empty name, which rendered as nothing, e.g. = '1'.

Tests

test/unit/Sql/PredicateIdentifierNameTest.php (#[Group('unit')]) uses data providers to cover:

  • broken forms now quoted as one identifier: hyphen, space, quote character, non-ASCII, tab, dotted hyphen, dotted space and hyphenated qualified star. Each runs across every predicate type above, plus where-array keys (with a value and with null), HAVING, Update WHERE and Delete WHERE;
  • valid forms proven byte-identical across the same paths;
  • an Argument::identifiers() list;
  • the shared TrustingHyphenWordPlatform;
  • rejected names: COUNT(id), a - b, x-y * 2, a+b on the hyphen platform, '' and a..b;
  • Expression, Literal and string predicates left unchanged.

Checks:

  • The full suite passes.
  • mago format --check, mago lint and mago analyze exit 0 on PHP 8.3, 8.4 and 8.5, and so does mago guard.

Select resolved each column string through quoteIdentifierInFragment(),
which splits on every non-word character, so a column such as x-y
rendered as "x""-""y" while its alias rendered as "x-y". Column strings
in the select list, join column lists and GROUP BY are now quoted as an
identifier chain split only on the table-qualifier dot, with a * part
left bare. A string holding an inline " AS " alias keeps fragment
quoting, as before.
A column string now switches to identifier-chain quoting only when
fragment quoting would render it broken: a piece between the safe
delimiters (space, "." and "*") that the platform would split and quote
in parts, as with x-y, a"b or COUNT(id), or two names separated by
spaces alone, as with "my col". Strings such as "price * qty" or "a*b"
render exactly as before.
…roken SQL

Predicate identifiers render through quoteIdentifierInFragment(), which
quotes every piece split out on a non-word character other than a space,
dot, * or AS. A name such as x-y, a"b, nâme or my col therefore renders
as adjacent quoted pieces or a bare sequence of identifiers.

Such names are now quoted whole as an identifier chain, split only on the
qualifier dot. Every name whose fragment quoting renders valid SQL keeps
its current rendering, judged against the platform's own fragment pattern.
@codecov

codecov Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

simon-mundy and others added 7 commits November 4, 2026 14:26
Replace Select's own column-quoting helpers with IdentifierQuoter, which
the ORDER BY and predicate fixes can share:

- A plain name of word characters and qualifier dots takes a fast path
  straight to fragment quoting, as before.
- Otherwise IdentifierShape reads the name in pieces. Fragment quoting is
  kept where it renders as written; a name it would split (x-y, my col,
  t.as) is quoted whole per qualifier part, with each part trimmed.
- Expression syntax that no identifier quoting renders as written, such
  as COUNT(id), a - b or x-y * 2, throws InvalidArgumentException, as
  does an empty name or qualifier part.
- An inline column alias such as 'x-y as z' renders "x-y" AS "z".

TrustingHyphenWordPlatform becomes the shared hyphen-as-word test
platform.
Replace quoteIdentifierArgument() with the shared IdentifierQuoter, so
identifier arguments follow the same rule as columns and ORDER BY names.
Expression syntax that no identifier quoting renders as written, such as
a+b or COUNT(id), and an empty name now throw InvalidArgumentException
instead of rendering broken SQL.
The test restricts coverage to the factories it lists, so the new
forEmptyIdentifierPart() and forUnquotableIdentifier() counted as
uncovered and failed codecov/patch.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant